escape crlf and quotes in multipart form-data field headers#2478
Open
metsw24-max wants to merge 1 commit into
Open
escape crlf and quotes in multipart form-data field headers#2478metsw24-max wants to merge 1 commit into
metsw24-max wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part-header injection in multipart serialisation
serialize_multipart_formdata_item_beginwrites each part'sContent-DispositionandContent-Typelines by concatenatingitem.name,item.filenameanditem.content_typestraight into the header, with no escaping. A name or filename carrying a CR/LF therefore injects further part headers (and a body), and one carrying a"closes the quoted-string early. When an application passes a value it received from elsewhere (a forwarded upload filename being the common case) the caller cannot prevent this from the outside, so the escaping belongs in the writer.A filename of
evil\r\nContent-Type: text/x-injected\r\n\r\nSMUGGLEDcurrently produces a part with an attacker-chosenContent-Typeand body;cli.Post("/x", items)is enough to reproduce it.The fix escapes CR, LF and
"to%0D/%0A/%22(RFC 7578 §5.1.1, matching the WHATWG form-data escaping browsers use) inside the serialiser, so every path that goes through it is covered. Well-formed values are unchanged. I have leant towards encoding rather than rejecting to keep the existing string-returning API and behaviour stable, though rejecting outright would also be defensible if you would prefer that.